feat(profiling): move flamegraph and differential flamegraph #30910
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JonasBa
force-pushed
the
jb/profiling/flamegraph-frame
branch
from
6bbc00f
to
cf99f30
Compare
JonasBa
force-pushed
the
jb/profiling/flamegraph-frame
branch
from
a5521ea
to
197c99d
Compare
size-limit report
|
Zylphrex
approved these changes
Jan 13, 2022
| const counts = new Map<string, number>(); | ||
|
|
||
| for (const frame of frames) { | ||
| const key = frame.frame.name + (frame.frame.file ? frame.frame.file : ''); |
There was a problem hiding this comment.
Should this be a method on Frame?
There was a problem hiding this comment.
@Zylphrex I thought about it before, but I haven't done it. The reason for that is that this is more the key that we use for color encoding which is not something that the Frame has to know or care about. Maybe a utility fn would be a good middle ground here, wdyt?
| this.configSpace = new Rect(0, 0, this.duration, this.depth); | ||
| } else { | ||
| // If we have no frames, set the trace duration to 1 second so that we can render a placeholder grid | ||
| this.configSpace = new Rect(0, 0, 1_000_000, 0); |
There was a problem hiding this comment.
Are the units always going to be microseconds? I see that it's possible to specify a unit in Profile. Should that be used to compute this value?
There was a problem hiding this comment.
Ah correct, we need to set that too, I missed that 🙏🏼
JonasBa
force-pushed
the
jb/profiling/flamegraph-frame
branch
from
e016734
to
b179054
Compare
JonasBa
force-pushed
the
jb/profiling/flamegraph-frame
branch
from
b179054
to
24788b6
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Port over the main flamegraph and differential flamegraph classes.
The two classes are used to build (not rendering) the flamegraph in different ways (see buildCallOrderGraph and buildLeftHeavyGraph). They are mainly responsible for outputting an array of frames to render and giving us a configuration view (usually at x:0, y:0, width: timeline duration, height: max stack height).
There are two main functions to look at - buildLeftHeavyGraph and buildCallOrderGraph.
They both iterate over all frames of a profile and append the frames to render to
this.frameswith the slight difference that buildLeftHeavyGraph sorts the tree first (depth first) and updates the frames startTime and endTime based on their new positions so that startTime is the sum of all child frames preceding the current frame.e.g. left heavy timings change
Note that the order of this.frames doesn't matter and it will be up to whoever consumes the flamegraph to determine what and when to render.